9796 continuing ica #10023
9796 continuing ica #10023
Conversation
Deploying agoric-sdk with
|
| Latest commit: |
2689372
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b3938cac.agoric-sdk.pages.dev |
| Branch Preview URL: | https://9796-continuing-ica.agoric-sdk.pages.dev |
turadg
force-pushed
the
9796-continuing-ica
branch
from
b931fa6
to
5304024
Compare
refs: #9796 ## Description #10023 started with a series of refactors and cleanup. This separates those into another PR to be reviewed more rapidly. It also includes a new feature to orchestrateAll to ease calling flows from flows. Each flow is available on the context object under `flows`. ### Security Considerations Every flow can call another flow. In the event a developer wants isolate between flows, they won't put them in the same `orchestrateAll`. ### Scaling Considerations n/a ### Documentation Considerations none ### Testing Considerations new tests ### Upgrade Considerations not yet deployed
turadg
force-pushed
the
9796-continuing-ica
branch
from
5304024
to
5b70d25
Compare
There was a problem hiding this comment.
Looks good, going to play around with it a bit more locally before approving since it's marked as Closes.
I see a clear path for sharing state from flow1 with subsequent flows. Wondering if it will cover state created in flow2 that's needed by flow3, e.g. the repeater in the restake contract.
| ); | ||
|
|
||
| return vowTools.watch(invP); | ||
| }, |
There was a problem hiding this comment.
Nice. We might want to include an example with a seat and offerArgs as well, something like:
DepositAndDelegate() {
const { account } = this.state;
const invP = zcf.makeInvitation(
(seat, delegations) =>
orchFns.depositAndDelegate(account, seat, delegations),
'Deposit and delegate',
undefined,
{
give: {
Stake: NatAmountShape,
},
},
);
return vowTools.watch(invP);
},There was a problem hiding this comment.
Aside - I was surprised to see we don't need to do seat.exit() (nor seat.tryExit() in the tests).
| const overrides = {}; | ||
| for (const invMakers of invitationMakers) { | ||
| // remove '__getInterfaceGuard__', '__getMethodNames__' | ||
| const names = getMethodNames(invMakers).filter(n => !n.startsWith('__')); |
There was a problem hiding this comment.
Nb: should we move this to @agoric/internal and call it getPublicMethodNames?
There was a problem hiding this comment.
no opinion here. WDYT @mhofman ?
There was a problem hiding this comment.
packages/orchestration/src/examples/continuing-invitation.contract.js
Outdated
Show resolved
Hide resolved
- Takes two or more InvitationMaker exos and combines them into a new one. - Useful for combining exos that are already instantiated
turadg
force-pushed
the
9796-continuing-ica
branch
from
5b70d25
to
2689372
Compare
| amount, | ||
| ) => { | ||
| console.log('depositAndDelegate', account, seat, validator, amount); | ||
| // TODO deposit the amount |
There was a problem hiding this comment.
Ticket # would be nice, but thanks for including this!
| }, | ||
| ); | ||
|
|
||
| return vowTools.watch(invP); |
There was a problem hiding this comment.
Thinking on this more, I don't think we need vowTools.watch here. We can expect the promise for an Invitation to resolve promptly - it's just the offer handler that needs to return a vow.
If we feel we should return a vow here, we should also make this change in the orchAccount exos. Currently, we're always returning a promise.
The only instance coming to mind where it'd be helpful to return a vow is if a developer wanted to call an invitationMaker from inside a flow - it'd need to be a vow to cross the membrane. I'm not sure how common this will be - we don't have any examples that need this currently.
There was a problem hiding this comment.
wanted to call an invitationMaker from inside a flow
I think that's what motivated the vow: an earlier version of this branch was calling between guest fns. But definitely not needed now. Thanks for pointing it out.
We can expect the promise for an Invitation to resolve promptly
I believe "promptly" means in "in the same run". zcf.makeInvitation calls out to another vat so I don't think it can be in the same run.
agoric-sdk/packages/zoe/src/contractFacet/zcfZygote.js
Lines 314 to 316 in daff9eb
refs: #10023 ## Description @0xpatrickdev points out it doesn't need to return a vow ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations n/a ### Testing Considerations Should not affect tests ### Upgrade Considerations undeployed
closes: #9796
Description
Demonstrates a pattern for creating continuing invitations that have handlers written as orchestration flows.
Builds upon the combine-invitation-makers from #9821
Security Considerations
None
Scaling Considerations
Augmenting an existing exo's invitationMaker requires two new ones: the exo holding the other makers and an exo to wrap them both.
Documentation Considerations
Once this lands it should have an explicit tutorial for developers needing this functionality.
Testing Considerations
new tests
Upgrade Considerations
not yet deployed